Skip to content

feat: add consumer/impact-traversal evidence to the shared review contract - #82

Merged
shaug merged 2 commits into
mainfrom
scott/52-consumer-impact-evidence
Jul 29, 2026
Merged

feat: add consumer/impact-traversal evidence to the shared review contract#82
shaug merged 2 commits into
mainfrom
scott/52-consumer-impact-evidence

Conversation

@shaug

@shaug shaug commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

  • Bump the shared review result contract to schema 1.2 (additive only): add
    a consumer_impact_evidence array recording, per changed shared symbol
    whose other call sites/consumers were traversed, the symbol, its defining
    location, one or more concrete consumer_search_evidence entries, and a
    disposition of all_consumers_consistent, inconsistency_found, or
    no_other_consumers.
  • review-suite/scripts/validate.py: enforce structure and non-emptiness
    only. Only correctness or aggregate results may carry this evidence. A
    disposition claiming other consumers were found
    (all_consumers_consistent, inconsistency_found) requires search
    evidence covering more than the changed symbol's own location;
    no_other_consumers requires at least one concrete search. The validator
    deliberately does not determine which changed symbols require an entry —
    it receives only a packet and a result, with no repository checkout to
    search, so it cannot itself decide whether a changed symbol has other call
    sites; that judgment belongs to the lens performing the traversal (Add correctness traversal and verification-sufficiency passes #53,
    not yet implemented). Extend the stale-schema-version rejection so a v1.1
    result now fails with a useful migration error, mirroring the existing
    v1.0 rejection.
  • Add a new canonical fixture, consumer-impact-traversal, modeling a
    sanitized version of the baseline's dependency-strictness-propagation
    miss, covering all 5 required fixture/test scenarios in
    review-suite/scripts/tests/test_contracts.py.
  • Migrate every canonical fixture, eval helper, and each bundled skill's
    standalone eval fixtures to schema 1.2; refresh the four bundled
    review-suite copies via just sync-contracts.
  • Document the validator's scope boundary explicitly in CONTRACT.md, the
    validator docstring, and a dedicated regression test, after a first
    review-code-change pass raised — and a second, clean pass confirmed
    resolved — the question of whether the validator should itself detect
    missing traversal from the diff (it should not: that would be exactly the
    independent correctness explorer / static call-graph tooling this
    ticket's non-goals rule out).

Why

Acceptance criteria (from #52's body)

  • Schema version 1.2 adds exactly the consumer_impact_evidence array
    described in the ticket; no changed-surface ledger, acceptance trace,
    or risk profile is added.
  • The validator requires a non-empty, structurally valid entry, and
    accepts a concretely-evidenced no_other_consumers disposition
    otherwise (structure and non-emptiness only — see the CONTRACT.md
    "Consumer/impact evidence" section for the validator's scope boundary
    and why determining which symbols require an entry is lens
    judgment, not validator judgment).
  • All required positive and negative fixtures exist (5 scenarios,
    covered by ConsumerImpactEvidenceTests in test_contracts.py).
  • Canonical and bundled contracts are byte-identical after
    just sync-contracts.
  • Schema 1.1 fixtures and callers are migrated atomically; stale
    documents (both 1.0 and now 1.1) fail with a useful error.
  • just format, just lint, and just test pass.
  • README and CHANGELOG describe the evidence addition and version
    transition (CHANGELOG updated; root README does not document schema
    versions, matching Make clean verdicts require passing validation and current-head lens evidence #51's own precedent).
  • No lens prompt, rubric, or orchestration behavior changes in this
    ticket.

Test plan

  • just format — all checks passed, no reformatting needed
  • just lint — all checks passed (skills-ref validation of all 7
    skills, plugin packaging validation)
  • just test — 234 tests in review-suite/scripts/tests (up from 227),
    plus every skill's own tests, all OK
  • just sync-contracts — all four bundled review-suite copies
    byte-identical to canonical source (verified by
    test_bundled_contracts.py and a direct cmp check)
  • Reviewed by the repository's own review-code-change suite (2 rounds;
    round 1 raised one correctness finding proposing the validator detect
    missing traversal from the diff — declined with evidence as exactly
    the independent-explorer/static-call-graph machinery this ticket's
    non-goals forbid, and strengthened documentation instead; round 2
    clean across solution-simplicity, correctness, and code-simplicity)

Scope note

This PR does not touch review-suite/evals/baseline/v1/ or
review-suite/evals/v2/ (frozen artifacts from #58/#59), does not touch
review-code-change/SKILL.md's orchestration matrix, the correctness
rubric, or any lens prompt, and does not add lens behavior that populates
consumer_impact_evidence — that is #53's scope.

Fixes #52

shaug and others added 2 commits July 28, 2026 19:59
…tract

## Summary
- Bump the shared review result contract to schema `1.2` (additive only): add
  a `consumer_impact_evidence` array recording, per changed shared symbol
  whose other call sites/consumers were traversed, the symbol, its defining
  location, one or more concrete `consumer_search_evidence` entries, and a
  `disposition` of `all_consumers_consistent`, `inconsistency_found`, or
  `no_other_consumers`.
- `review-suite/scripts/validate.py`: enforce structure and non-emptiness only
  — the validator does not determine which changed symbols require an entry
  (that is lens judgment, owned by a later child). A disposition claiming
  other consumers were found (`all_consumers_consistent`,
  `inconsistency_found`) requires search evidence covering more than the
  changed symbol's own location; `no_other_consumers` requires at least one
  concrete search. Only `correctness` or `aggregate` results may carry this
  evidence. Extend the stale-schema-version rejection so a v1.1 result now
  fails with a useful migration error, mirroring the existing v1.0 rejection.
- Add a new canonical fixture, `consumer-impact-traversal`, modeling a
  sanitized version of the baseline's `dependency-strictness-propagation`
  miss (a changed shared helper with a sibling call site, plus a second
  changed private helper used from exactly one call site), covering all 5
  required fixture/test scenarios in `review-suite/scripts/tests/test_contracts.py`.
- Migrate every canonical fixture, eval helper, and each bundled skill's
  standalone eval fixtures to schema 1.2; refresh the four bundled
  `review-suite` copies via `just sync-contracts`.

## Why
- #52 (narrowed by #59's decision record): the baseline's
  `dependency-strictness-propagation` case shows 5 of 5 attempts missed the
  same sibling call site of a changed shared helper, because nothing in the
  contract required a reviewer to record whether other call sites were
  traversed. This ticket owns only the schema and validator for that
  evidence; it does not add the lens behavior that populates it (#53), and it
  does not add the changed-surface ledger, acceptance trace, or risk profile
  #59 explicitly dropped as unsupported by baseline evidence.

## Verification
- `just format`, `just lint`, and `just test` all pass (234 tests in
  `review-suite/scripts/tests`, up from 227; 6 new consumer-impact-evidence
  tests plus one new stale-v1.1 rejection test).
- `just sync-contracts` leaves all four bundled `review-suite` copies
  byte-identical to the canonical source (verified by
  `test_bundled_contracts.py` and a direct `cmp` check).
- No file under `review-suite/evals/baseline/v1/` or `review-suite/evals/v2/`
  was touched; `review-code-change/SKILL.md`'s orchestration matrix, the
  correctness rubric, and every lens prompt are unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Summary
- Strengthen `review-suite/CONTRACT.md`, `validate.py`'s
  `_check_consumer_impact_evidence` docstring, and its regression test to
  explain, with concrete reasoning, why the validator never inspects the
  packet diff to decide whether a changed symbol needed a
  `consumer_impact_evidence` entry: this validator receives only a packet and
  a result with no repository checkout to search, so it cannot itself
  determine whether a changed symbol has other call sites, and adding that
  determination here would be the independent correctness explorer and
  static call-graph tooling #52's non-goals rule out.

## Why
- A fresh `review-code-change` pass on the prior candidate raised a blocking
  correctness finding proposing that `validate_pair` be extended to detect,
  from the diff, whether a changed symbol's sibling call site was also
  covered. That proposed mechanism is exactly the independent
  correctness-explorer / static-call-graph tooling #52's own non-goals
  forbid, and #52's own text assigns "which changed symbols require an
  entry" to "the correctness lens's own traversal pass (#53)" — a mechanism
  that does not exist yet in this candidate — not to this validator. The
  real baseline miss this evidence exists to surface
  (`dependency-strictness-propagation`) involved a sibling call site the diff
  never touched, which only live repository access (available to the
  reviewing agent, never to this validator) can find; a diff-only heuristic
  would not even correctly model that failure mode. Declining to build that
  mechanism is therefore correct, but the prior candidate's reasoning for the
  boundary was implicit; this revision makes it explicit and self-evident
  from the artifacts themselves.

## Verification
- `just format`, `just lint`, and `just test` all pass (234 tests in
  `review-suite/scripts/tests`, unchanged in count from the prior commit —
  this revision only strengthens documentation and comments, no schema,
  validator behavior, or fixture changed).
- `just sync-contracts` leaves all four bundled `review-suite` copies
  byte-identical to the canonical source.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shaug
shaug merged commit 8e4fdbd into main Jul 29, 2026
1 check passed
@shaug
shaug deleted the scott/52-consumer-impact-evidence branch July 29, 2026 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add consumer/impact-traversal evidence to the shared review contract

1 participant